pkg/cli/admin/mustgather/mustgather_test.go (1)

790-801: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Compare the complete namespace annotation map.

The current assertions only inspect one key through multiple conditionals, so regressions to the other annotations would go unnoticed. Build the expected annotation map for each case and compare it with cmp.Diff.

Proposed test adjustment

-			_, exists := ns.Annotations[namespaceWorkloadAllowedAnnotation]
-			if tc.expectAnnotation && !exists {
-				t.Errorf("expected %s annotation, got annotations: %v", namespaceWorkloadAllowedAnnotation, ns.Annotations)
-			}
-			if !tc.expectAnnotation && exists {
-				t.Errorf("did not expect %s annotation when partitioning is disabled", namespaceWorkloadAllowedAnnotation)
-			}
-			if tc.expectAnnotation && ns.Annotations[namespaceWorkloadAllowedAnnotation] != "management" {
-				t.Errorf("expected annotation value %q, got %q", "management", ns.Annotations[namespaceWorkloadAllowedAnnotation])
+			expectedAnnotations := map[string]string{
+				"oc.openshift.io/command":    "oc adm must-gather",
+				"openshift.io/node-selector": "",
+			}
+			if tc.expectAnnotation {
+				expectedAnnotations[namespaceWorkloadAllowedAnnotation] = "management"
+			}
+			if diff := cmp.Diff(expectedAnnotations, ns.Annotations); diff != "" {
+				t.Errorf("namespace annotations differ (-want +got):\n%s", diff)
 			}

As per coding guidelines, tests should assemble expected objects and compare them using google/go-cmp.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/cli/admin/mustgather/mustgather_test.go` around lines 790 - 801, Update
the table-driven test around newNamespace to construct the complete expected
annotation map for each test case and compare it with ns.Annotations using
cmp.Diff. Remove the individual namespaceWorkloadAllowedAnnotation existence and
value conditionals, while preserving each case’s expected annotations.

Source: Coding guidelines

pkg/cli/admin/mustgather/mustgather.go (1)

322-322: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document all changed Go functions.

The changed declarations omit required descriptive comments:

• pkg/cli/admin/mustgather/mustgather.go#L322-L322: document the Infrastructure lookup and fail-open behavior.
• pkg/cli/admin/mustgather/mustgather.go#L1127-L1127: document conditional namespace workload annotation.
• pkg/cli/admin/mustgather/mustgather_test.go#L725-L725: document the detection test.
• pkg/cli/admin/mustgather/mustgather_test.go#L773-L773: document the namespace annotation test.

As per coding guidelines, all exported and unexported Go functions must have descriptive comments.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/cli/admin/mustgather/mustgather.go` at line 322, Document all four
changed Go functions with concise comments: in
pkg/cli/admin/mustgather/mustgather.go lines 322-322, describe
isCPUPartitioningEnabled’s Infrastructure lookup and fail-open behavior; in
lines 1127-1127, describe the conditional namespace workload annotation; in
pkg/cli/admin/mustgather/mustgather_test.go lines 725-725 and 773-773, describe
the CPU partitioning detection and namespace annotation tests respectively.

Source: Coding guidelines